Data to csv #1900

akp2603 · 2020-10-10T16:09:49Z

Description

The following PR includes backend APIs for the aforementioned issues.

More Details

Provided 3 apis. One to generate a request for data to be generated, one to fetch the status of the request being queued(CSV generation happens async, hence this api for polling.), and last one to download the file.
The following PR enables a user to generate a csv to get his/her data. With every new csv generated, the old csv get deleted, to prevent disk space.
Files, 30 days old, will be deleted through a cron.

Corresponding Issue

#1560

Reviewing this pull request? Check out our Code Review Practices guide if you haven't already!

welcome · 2020-10-10T16:09:51Z

Thank you for opening this pull request with us! Be sure to follow our Pull Request Practices. Let us know if you have any questions on Slack.

bennpham

Hi Aashish, this PR looks nice for a proof of concept for a job scheduler or data backup. Good work on it 👍 . If I made a scheduler, I'd technically store it on Amazon S3 or Google Cloud service instead where data storage would be cheaper and less costlier than storing everything on disc. Especially when more users were to register, it'd be difficult to scale.

On the other hand, for an actual csv download itself, I'm wondering but couldn't you just do the following (modify below to restrict it to only users who owned that data):

Model.rb

def self.to_csv
  heading = ["Col1", "Col2", "Col3]

  CSV.generate do |csv|
    csv << heading
      
    Model.all.each do |model|
      csv << [model.data1, model.data2, model.data3]
    end
end

Controller.rb

def index
  ...

  respond_to do |format|
    format.html
    format.csv {
      send_data Model.to_csv, filename: "model-data-#{Date.current.strftime('%Y%m%d')}.csv"
    }
  end
end

index.html.erb

<%= link_to "Export to CSV", { format: :csv }, class: "btn" %>

akp2603 · 2020-10-11T15:40:59Z

If I made a scheduler, I'd technically store it on Amazon S3 or Google Cloud service instead where data storage would be cheaper and less costlier than storing everything on disc. Especially when more users were to register, it'd be difficult to scale.

Hi @bennpham firstly thanks for taking the time to review my PR. Really appreciated. Secondly, regarding what you said, I second this, but I asked @julianguyen regarding the same before implementing it. I suggested what you suggested. But she preferred it storing it on the disk only. Hence I implemented it this way. Maybe we can switch to S3 as an enhancement over this?

Also, regarding the csv file, what you've suggested is separated files for separate models. But I implemented it all in a single file. (The format for which, was again, confirmed with Julia.). Can share it with you on slack too, if you want.

Please let me know if you still want any changes regarding the same.

julianguyen

I don't have deep expertise this kind of data storage, so I'm definitely open to whatever option is free 🙃 Just cause we have limited funds at the moment as an open source project.

julianguyen · 2020-10-12T01:42:06Z

Gemfile

@@ -52,6 +52,14 @@ gem 'selenium-webdriver', '~> 3.142.3'

 gem 'rubyzip', '~> 1.3.0'

+gem 'sidekiq', '5.0.5'
+# Uniqueness in sidekiq jobs


Thanks for adding comments but I don't think they're absolutely necessary!

julianguyen · 2020-10-12T01:43:56Z

app/assets/stylesheets/users/reports.scss

@@ -0,0 +1,3 @@
+// Place all the styles related to the Users::Reports controller here.


Let's delete this file since it's unused!

julianguyen · 2020-10-12T02:01:17Z

app/models/allyship.rb

@@ -12,6 +12,14 @@
 #

 class Allyship < ApplicationRecord
+  DISPLAY_ATTRIBUTES = %w[


Let's rename this to something like USER_DATA_ATTRIBUTES so that it's more obvious that this is being used for that.

julianguyen · 2020-10-12T02:27:08Z

In terms of recommendations for learning how to test, I recommend looking at our existing specs for general patterns and ideas. The official RoR docs has a good starter doc on testing.

Feel free to ask for help in our #dev channel on Slack!

julianguyen · 2020-10-17T21:45:01Z

config/env/development.example.env

@@ -39,3 +39,7 @@ RAISE_DELIVERY_ERRORS="false"
 PSQL_HOST=""
 PSQL_USERNAME=""
 PSQL_PASSWORD=""
+
+# REDIS


You'll want to add this to config/env/test.example.env as well.

julianguyen

Strong work! Thanks for looking into writing tests :D

julianguyen · 2020-10-18T01:14:22Z

app/helpers/users/reports_helper.rb

+    end
+
+    def fetch_request_status_helper(user, request_id)
+      return 400, { error: 'Request id can be blank.' } if request_id.blank?


Should this be "Request id can't be blank" instead?

julianguyen · 2020-10-18T01:14:33Z

app/helpers/users/reports_helper.rb

+    end
+
+    def download_data_helper(user, request_id)
+      return 400, { error: 'Request id can be blank.' } if request_id.blank?


Same question as above.

julianguyen · 2020-10-18T01:21:06Z

app/workers/process_data_request_worker.rb

+
+  def perform(request_id)
+    data_request = Users::DataRequest.find_by(request_id: request_id)
+    return if data_request.blank?


Lines 8 and 9 could be combined with an ||.

julianguyen · 2020-10-18T01:23:43Z

spec/models/users/data_request_spec.rb

+
+    it 'is invalid without a status_id' do
+      data_request = build(:partial_data_request)
+      expect(data_request).to have(2).error_on(:status_id) 


What are there two errors?

presence.

inclusion.
Both are treated separatedly. There are 2 validations on status_id. nil is blank and also not amongst the included values.

julianguyen · 2020-10-18T01:26:24Z

spec/requests/users/reports_request_spec.rb

+      before { sign_in user }
+
+      it 'creates a data download request' do
+        post "/users/data"


Let's replace this and other occurrences with users_data_path.

julianguyen · 2020-10-18T01:26:52Z

spec/requests/users/reports_request_spec.rb

+      before { sign_in user }
+
+      it 'fetches the status of data request with a blank request_id' do
+        get "/users/data/status"


Let's replace this and other occurrences with users_data_status_path.

julianguyen · 2020-10-18T01:27:52Z

spec/requests/users/reports_request_spec.rb

+      before { sign_in user }
+
+      it 'fetches the file with a blank request_id' do
+        get "/users/data/download"


Let's replace this and other occurrences with users_data_download_path.

julianguyen

Fantastic work! Thanks so much for taking this on and going through all the review cycles here :)

welcome · 2020-10-23T19:30:44Z

Thank you for merging this pull request with us! If you haven't already, in another pull request, please add yourself to our About page.

Aashish Passrija added 2 commits October 10, 2020 21:31

Data Export To CSV for user : backend apis

73fd907

Merge branch 'main' of github.com:ifmeorg/ifme into main

7b9d389

akp2603 added wip hacktoberfest labels Oct 10, 2020

julianguyen self-requested a review October 10, 2020 19:10

bennpham reviewed Oct 11, 2020

View reviewed changes

Aashish Passrija added 2 commits October 11, 2020 21:59

rubocop

e7b8fe5

Merge branch 'main' of github.com:ifmeorg/ifme into data_to_csv

28d1c90

julianguyen reviewed Oct 12, 2020

View reviewed changes

akp2603 and others added 3 commits October 15, 2020 23:32

Delete reports.scss

3c0195b

changed variable name

f88ab4a

Merge branch 'data_to_csv' of github.com:ifmeorg/ifme into data_to_csv

68034c4

akp2603 added the hacktoberfest-accepted label Oct 16, 2020

Aashish Passrija and others added 7 commits October 17, 2020 04:33

request spec added

aebe22b

Delete delete_stale_data_worker_spec.rb

38cd98b

Delete process_data_request_worker_spec.rb

e3546db

Delete reports_helper_spec.rb

12c1a64

data request model specs added

1967ff8

Merge branch 'data_to_csv' of github.com:ifmeorg/ifme into data_to_csv

ac9fdea

rubocop fixes

aa369ed

akp2603 removed the wip label Oct 17, 2020

Aashish Passrija and others added 4 commits October 18, 2020 02:28

refactoring

9b2fa1f

refactoring

47b0af0

Delete delete_stale_data_worker_spec.rb

79085ff

Delete process_data_request_worker_spec.rb

27a6b7d

julianguyen reviewed Oct 17, 2020

View reviewed changes

added test.example.env

cbf22e1

Aashish Passrija and others added 3 commits October 18, 2020 03:17

Merge branch 'data_to_csv' of github.com:ifmeorg/ifme into data_to_csv

342e0d3

Setup Redis in Circle CI config

9e04274

Update config.yml

b1502a6

julianguyen reviewed Oct 18, 2020

View reviewed changes

Aashish Passrija added 3 commits October 18, 2020 08:07

review changes

ba3c682

Merge branch 'data_to_csv' of github.com:ifmeorg/ifme into data_to_csv

31f18b2

Merge branch 'main' of github.com:ifmeorg/ifme into data_to_csv

f92ba10

julianguyen approved these changes Oct 23, 2020

View reviewed changes

julianguyen merged commit 1eb6d8b into main Oct 23, 2020

julianguyen deleted the data_to_csv branch October 23, 2020 19:30

julianguyen mentioned this pull request Oct 23, 2020

Functionality to download your user data #1560

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data to csv #1900

Data to csv #1900

akp2603 commented Oct 10, 2020 •

edited

Loading

welcome bot commented Oct 10, 2020

bennpham left a comment •

edited

Loading

akp2603 commented Oct 11, 2020

julianguyen left a comment

julianguyen Oct 12, 2020

julianguyen Oct 12, 2020

julianguyen Oct 12, 2020

julianguyen commented Oct 12, 2020

julianguyen Oct 17, 2020

akp2603 Oct 17, 2020

julianguyen left a comment

julianguyen Oct 18, 2020

julianguyen Oct 18, 2020

julianguyen Oct 18, 2020

julianguyen Oct 18, 2020

akp2603 Oct 18, 2020

julianguyen Oct 18, 2020

julianguyen Oct 18, 2020 •

edited

Loading

julianguyen Oct 18, 2020 •

edited

Loading

julianguyen left a comment

welcome bot commented Oct 23, 2020

		@@ -0,0 +1,3 @@
		// Place all the styles related to the Users::Reports controller here.

Data to csv #1900

Data to csv #1900

Conversation

akp2603 commented Oct 10, 2020 • edited Loading

Description

More Details

Corresponding Issue

welcome bot commented Oct 10, 2020

bennpham left a comment • edited Loading

Choose a reason for hiding this comment

akp2603 commented Oct 11, 2020

julianguyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianguyen commented Oct 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianguyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianguyen Oct 18, 2020 • edited Loading

Choose a reason for hiding this comment

julianguyen Oct 18, 2020 • edited Loading

Choose a reason for hiding this comment

julianguyen left a comment

Choose a reason for hiding this comment

welcome bot commented Oct 23, 2020

akp2603 commented Oct 10, 2020 •

edited

Loading

bennpham left a comment •

edited

Loading

julianguyen Oct 18, 2020 •

edited

Loading

julianguyen Oct 18, 2020 •

edited

Loading